Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OpenCLIP sample for visual question answering #977

Merged
merged 10 commits into from
Jul 10, 2024
Merged

OpenCLIP sample for visual question answering #977

merged 10 commits into from
Jul 10, 2024

Conversation

vidyasiv
Copy link
Contributor

@vidyasiv vidyasiv commented May 13, 2024 •
edited
Loading

What does this PR do?

Framework HPU Precision Command Inference time per iteration Throughput
Pytorch 2.2.0 1 BF16 python run_openclip_vqa.py --bf16 --use_hpu_graphs 12.44ms 723.7 images/s

Fixes # (issue)

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

Co- author: @imangohari1 (Gaudi1 CI results)

@vidyasiv vidyasiv marked this pull request as ready for review May 14, 2024 00:31
@vidyasiv vidyasiv requested a review from regisss as a code owner May 14, 2024 00:31
skavulya
skavulya reviewed May 17, 2024
View reviewed changes
Copy link
Contributor

@skavulya skavulya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vidyasiv Thanks for the contribution

examples/visual-question-answering/README.md Outdated Show resolved Hide resolved
examples/visual-question-answering/run_openclip_vqa.py Show resolved Hide resolved
tests/test_openclip_vqa.py Show resolved Hide resolved
@vidyasiv vidyasiv requested a review from skavulya May 31, 2024 18:30
skavulya
skavulya approved these changes Jun 11, 2024
View reviewed changes
Copy link
Contributor

@skavulya skavulya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @vidyasiv

imangohari1
imangohari1 suggested changes Jun 18, 2024
View reviewed changes
Copy link
Contributor

@imangohari1 imangohari1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vidyasiv
Hi Vidya,
Thank you for opening this PR.
Some suggestions/comments here and rest as needed in the specific lines.

  • I recommend re-syncing this branch to the top of main
  • please do a make style as I can see some formatting changes required.
  • I suggest adding dtype to the new funcs (i.e. postprocess) inputs and outputs.
  • We likely need to measure these WLs on Gaudi1 for completeness of ci testings.

imangohari1
imangohari1 reviewed Jun 18, 2024
View reviewed changes
Copy link
Contributor

@imangohari1 imangohari1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More comments in below.

examples/visual-question-answering/README.md

```bash
pip install -r openclip_requirements.txt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I am getting very large throughput.
is this even a realistic usecase?

06/18/2024 16:49:42 - INFO - __main__ - Running warmup
06/18/2024 16:49:43 - INFO - __main__ - Running inference
06/18/2024 16:49:43 - INFO - __main__ - Inference Time per iteration = 5.014ms
06/18/2024 16:49:43 - INFO - __main__ - Throughput = 1.795e+03 images/s

Copy link
Contributor Author

@vidyasiv vidyasiv Jun 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does seem high..I don't recall seeing that large a throughput

examples/visual-question-answering/README.md Show resolved Hide resolved
examples/visual-question-answering/README.md
```

Models that have been validated:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely needs to be valided on Gaudi1 as well.

examples/visual-question-answering/run_openclip_vqa.py Outdated Show resolved Hide resolved
tests/test_openclip_vqa.py Show resolved Hide resolved
tests/test_openclip_vqa.py Show resolved Hide resolved
@vidyasiv @imangohari1
updates per PR review
15fc3dc 
Co-Authored-By: imangohari1 <[email protected]>
imangohari1
imangohari1 approved these changes Jun 20, 2024
View reviewed changes
Copy link
Contributor

@imangohari1 imangohari1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@regisss
Could you please take a look at this? We've done some testing on this and the results are given in above.

@libinta libinta added the run-test Run CI for PRs from external contributors label Jul 6, 2024
@HuggingFaceDocBuilderDev
Copy link

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@regisss regisss merged commit afb8e8e into huggingface:main Jul 10, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@imangohari1 imangohari1 imangohari1 approved these changes

@regisss regisss regisss approved these changes

@skavulya skavulya skavulya approved these changes

Assignees
No one assigned
Labels
run-test Run CI for PRs from external contributors
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@vidyasiv @HuggingFaceDocBuilderDev @skavulya @regisss @imangohari1 @libinta